London | 26-ITP-May | Dipa Sarker | Sprint 3 | Practice TDD#1471
London | 26-ITP-May | Dipa Sarker | Sprint 3 | Practice TDD#1471Dipa-Sarker wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Great suite of test cases, can we think of some more edge cases to test, making sure the function can handle tricky arguments?
There was a problem hiding this comment.
Thank you for your review. I have added several edge cases to the tests. Please let me know if I've understood your question correctly.
There was a problem hiding this comment.
Great Dipa yeah this is exactly what I meant, here you are testing all possible edge cases
There was a problem hiding this comment.
Readability and naming are spot on!
I'm curious about the control flow choices here. Could you share why you opted for the else if chain and wrapped the final return in an else block?
There was a problem hiding this comment.
There are multiple conditions to check that's why I used else if inside if...else statement. I wrapped the final return in else cause none of the above conditions are true for "th", so for "th" else block will execute. That's my thought actually.
There was a problem hiding this comment.
That completely makes sense and your logic works perfectly, but we can still make it a bit simpler.
-
We can avoid the separate first
ifblock and check for the exceptions directly inside each rule using the&&operator.
Something like this:if (lastDigit === 1 && lastTwoDigits !== 11) return \${num}st;`` -
When an
ifstatement contains areturn, you can safely drop the "else" fromelse ifto make it more readable.
Becausereturnstops the function entirely, if allifs fail, the program naturally drops down to the lines below. This means we can remove the finalelsewrapper as well and just leave the 'th' return at the very bottom as our default."
Does this make sense to you?
There was a problem hiding this comment.
oh! yeah I got your point. Should I have to change the style of the code?
Learners, PR Template
Self checklist
Changelist